-
Notifications
You must be signed in to change notification settings - Fork 492
chore(config/tracer): migrate debugAbandonedSpans #4221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
BenchmarksBenchmark execution time: 2025-12-05 18:45:54 Comparing candidate commit 44f2aa9 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics. |
| t.Run("default", func(t *testing.T) { | ||
| assert := assert.New(t) | ||
| assert.False(tracer.config.debugAbandonedSpans) | ||
| assert.False(tracer.config.internalConfig.DebugAbandonedSpans()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(For here and every other place) is it ever possible for internalConfig on the tracer.config to be nil? Or for config itself to be nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible, yes, if tracer has not initialized internalConfig via the Get() function, but Config is designed with the expectations that callers must use Get() to get the instance before calling getters/setters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alternative is to make every getter/setter on Config check if the receiver c is not nil, but I think it's more standard Golang to design under the assumptions described above.
What does this PR do?
Tracer migrates to using Config.debugAbandonedSpans instead of its own config.debugAbandonedSpans.
Motivation
https://datadoghq.atlassian.net/browse/APMAPI-1748
Reviewer's Checklist
./scripts/lint.shlocally.Unsure? Have a question? Request a review!